Update `azurerm_api_management` - Support more cipher options #9276

flo-02-mu · 2020-11-11T21:41:55Z

Cipher options that can be disabled according to https://docs.microsoft.com/en-us/rest/api/apimanagement/2019-12-01/apimanagementservice/update#apimanagementserviceupdateparameters are added. As most of them are considered to be insecure, they are defaulting to false if not enabled.

katbyte · 2020-11-12T01:08:10Z

azurerm/internal/services/apimanagement/api_management_resource.go

@@ -294,6 +303,52 @@ func resourceArmApiManagementService() *schema.Resource {
 							Optional: true,
 							Default:  false,
 						},
+
+						"enable_tls_ecdhe_ecdsa_with_aes256_cbc_sha_ciphers": {


Could we put enabled on the end of all these properties? this is how we name booleans now:

Suggested change

"enable_tls_ecdhe_ecdsa_with_aes256_cbc_sha_ciphers": {

"tls_ecdhe_ecdsa_with_aes256_cbc_sha_ciphers_enabled": {

Done. Note: I renamed also the one cipher that was already there (enable_triple_des_ciphers) to have at least all cipher flags consistent. Technically this is a breaking change then.

# Conflicts: # azurerm/internal/services/apimanagement/api_management_resource.go

manicminer · 2020-11-23T12:28:50Z

azurerm/internal/services/apimanagement/api_management_resource.go

@@ -291,7 +300,53 @@ func resourceArmApiManagementService() *schema.Resource {
 							Default:  false,
 						},

-						"enable_triple_des_ciphers": {


@flo-02-mu You can maintain compatibility by keeping the existing property, adding the Deprecated field, and checking for both in the expandApiManagementCustomProperties() function (preferring the new one).

Added. Not sure if the d.GetOk(...) works for the nested block as expected though.

…flags

flo-02-mu · 2021-01-11T07:40:27Z

@manicminer , @katbyte Any chance to get this one pushed?

manicminer · 2021-01-11T07:42:31Z

@flo-02-mu If you can resolve the merge conflicts I'll take another look, thanks!

…flags # Conflicts: # azurerm/internal/services/apimanagement/api_management_resource.go

flo-02-mu · 2021-01-11T09:50:33Z

@flo-02-mu If you can resolve the merge conflicts I'll take another look, thanks!

@manicminer Sorry, I did not realize that. It's updated now.

manicminer

@flo-02-mu Thanks for that, this is mostly looking good. I've made some suggestions, some are necessary for the renamed property to work. Once these are addressed this should be good to merge.

azurerm/internal/services/apimanagement/api_management_resource.go

website/docs/r/api_management.html.markdown

…e.go Co-authored-by: Tom Bamford <[email protected]>

Co-authored-by: Tom Bamford <[email protected]>

manicminer

Thanks @flo-02-mu, LGTM! Awaiting final test results and then we can merge this.

manicminer

@flo-02-mu The TestAccApiManagement_complete is now passing, however there's an issue with the APIM Consumption SKU which doesn't support custom ciphers. These likely need to be conditionally omitted from the request.

Error: creating/updating API Management Service "acctestAM-210114203058931793" (Resource Group "acctestRG-210114203058931793"): apimanagement.ServiceClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="NotSupported" Message="'Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_128_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_128_CBC_SHA256,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_128_GCM_SHA256,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_256_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_256_CBC_SHA256,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TripleDes168' customProperties are not supported in SkuType Consumption."

flo-02-mu · 2021-01-16T18:41:24Z

@flo-02-mu The TestAccApiManagement_complete is now passing, however there's an issue with the APIM Consumption SKU which doesn't support custom ciphers. These likely need to be conditionally omitted from the request.

Error: creating/updating API Management Service "acctestAM-210114203058931793" (Resource Group "acctestRG-210114203058931793"): apimanagement.ServiceClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="NotSupported" Message="'Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_128_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_128_CBC_SHA256,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_128_GCM_SHA256,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_256_CBC_SHA,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TLS_RSA_WITH_AES_256_CBC_SHA256,Microsoft.WindowsAzure.ApiManagement.Gateway.Security.Ciphers.TripleDes168' customProperties are not supported in SkuType Consumption."

@manicminer I included all cipher options inside the conditional SKU block. Where can I check the test results?

manicminer · 2021-01-18T07:26:17Z

Thanks @flo-02-mu, this LGTM. I'm re-running the tests now; they aren't public at this time since we run them with private Azure credentials.

manicminer

Tests passing (unrelated keyvault failures)

ghost · 2021-01-21T16:32:52Z

This has been released in version 2.44.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.44.0"
}
# ... other configuration ...

ghost · 2021-02-17T14:50:19Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

flo-02-mu added 2 commits November 11, 2020 22:26

Add more cipher options to API management.

fc4a449

Add new cipher options to acc test.

aa8fd44

ghost added size/L documentation labels Nov 11, 2020

Rename cipher options to match allowed pattern

29972d5

katbyte reviewed Nov 12, 2020

View reviewed changes

Rename all cipher flags to match proposed convention.

613fea4

jackofallops added enhancement service/api-management labels Nov 13, 2020

Merge branch 'master' into more-apim-cipher-flags

ff22d44

# Conflicts: # azurerm/internal/services/apimanagement/api_management_resource.go

manicminer reviewed Nov 23, 2020

View reviewed changes

manicminer added the waiting-response label Nov 25, 2020

flo-02-mu added 3 commits November 28, 2020 22:23

Add deprecation fallback for enable_triple_des_ciphers

633c21a

Fix path for deprecated triple des property

db4e4bb

Merge remote-tracking branch 'upstream/master' into more-apim-cipher-…

89d4e55

…flags

ghost removed the waiting-response label Jan 11, 2021

Merge remote-tracking branch 'upstream/master' into more-apim-cipher-…

002ef1b

…flags # Conflicts: # azurerm/internal/services/apimanagement/api_management_resource.go

manicminer suggested changes Jan 11, 2021

View reviewed changes

manicminer assigned flo-02-mu Jan 11, 2021

manicminer added the waiting-response label Jan 11, 2021

manicminer added this to the v2.43.0 milestone Jan 11, 2021

flo-02-mu and others added 6 commits January 12, 2021 14:17

Update azurerm/internal/services/apimanagement/api_management_resourc…

946499d

…e.go Co-authored-by: Tom Bamford <[email protected]>

Update azurerm/internal/services/apimanagement/api_management_resourc…

c57121f

…e.go Co-authored-by: Tom Bamford <[email protected]>

Update azurerm/internal/services/apimanagement/api_management_resourc…

91acc48

…e.go Co-authored-by: Tom Bamford <[email protected]>

Update azurerm/internal/services/apimanagement/api_management_resourc…

cd34408

…e.go Co-authored-by: Tom Bamford <[email protected]>

Update azurerm/internal/services/apimanagement/api_management_resourc…

937847a

…e.go Co-authored-by: Tom Bamford <[email protected]>

Remove non-existing property from documentation.

4a8635b

Update website/docs/r/api_management.html.markdown

841568f

Co-authored-by: Tom Bamford <[email protected]>

tombuildsstuff modified the milestones: v2.43.0, v2.44.0 Jan 14, 2021

tombuildsstuff requested a review from manicminer January 14, 2021 12:05

manicminer approved these changes Jan 14, 2021

View reviewed changes

azurerm_api_management: fix typo in test config

668b0a1

manicminer suggested changes Jan 14, 2021

View reviewed changes

Exclude all cipher options for consumption sku

22e4937

ghost removed the waiting-response label Jan 16, 2021

azurerm_api_management: use Computed for deprecated attribute

3cfe3db

manicminer approved these changes Jan 18, 2021

View reviewed changes

manicminer merged commit 5b6d78e into hashicorp:master Jan 18, 2021

manicminer added a commit that referenced this pull request Jan 18, 2021

Update changelog for #9276

4637173

flo-02-mu deleted the more-apim-cipher-flags branch January 18, 2021 21:25

ghost locked as resolved and limited conversation to collaborators Feb 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `azurerm_api_management` - Support more cipher options #9276

Update `azurerm_api_management` - Support more cipher options #9276

flo-02-mu commented Nov 11, 2020

katbyte Nov 12, 2020

flo-02-mu Nov 12, 2020

manicminer Nov 23, 2020 •

edited

Loading

flo-02-mu Nov 29, 2020

flo-02-mu commented Jan 11, 2021

manicminer commented Jan 11, 2021

flo-02-mu commented Jan 11, 2021

manicminer left a comment

manicminer left a comment

manicminer left a comment •

edited

Loading

flo-02-mu commented Jan 16, 2021

manicminer commented Jan 18, 2021

manicminer left a comment

ghost commented Jan 21, 2021

ghost commented Feb 17, 2021

	"enable_tls_ecdhe_ecdsa_with_aes256_cbc_sha_ciphers": {
	"tls_ecdhe_ecdsa_with_aes256_cbc_sha_ciphers_enabled": {

Update azurerm_api_management - Support more cipher options #9276

Update azurerm_api_management - Support more cipher options #9276

Conversation

flo-02-mu commented Nov 11, 2020

katbyte Nov 12, 2020

Choose a reason for hiding this comment

flo-02-mu Nov 12, 2020

Choose a reason for hiding this comment

manicminer Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

flo-02-mu Nov 29, 2020

Choose a reason for hiding this comment

flo-02-mu commented Jan 11, 2021

manicminer commented Jan 11, 2021

flo-02-mu commented Jan 11, 2021

manicminer left a comment

Choose a reason for hiding this comment

manicminer left a comment

Choose a reason for hiding this comment

manicminer left a comment • edited Loading

Choose a reason for hiding this comment

flo-02-mu commented Jan 16, 2021

manicminer commented Jan 18, 2021

manicminer left a comment

Choose a reason for hiding this comment

ghost commented Jan 21, 2021

ghost commented Feb 17, 2021

Update `azurerm_api_management` - Support more cipher options #9276

Update `azurerm_api_management` - Support more cipher options #9276

manicminer Nov 23, 2020 •

edited

Loading

manicminer left a comment •

edited

Loading